Conversation
8f9dd47 to
5cd3619
Compare
| if isinstance(kind, tuple): | ||
| if device is None: | ||
| device = CPU_DEVICE | ||
| if isinstance(kind, type(None) | str): |
There was a problem hiding this comment.
What does this check? I was expecting to read isinstance(kind, (type(None), str)) instead of seeing a |. For my education, what is the difference/when would I use one and when the other?
There was a problem hiding this comment.
It's a union type, https://peps.python.org/pep-0604/. AFAIU the two isinstance forms are exactly equivalent.
FWIW, I also expect(ed) a tuple of types but apparently multiple linters pick on this in their "modernization" recommendations, similar to % string formatting. And some scipy reviewers felt strongly enough --- the sum total effect is apparently that my finger memory changed from (int, float) to int | float.
There was a problem hiding this comment.
Every day is a school day! Thanks for the explanation
There was a problem hiding this comment.
Shouldn't it be isinstance(kind, None | str) instead of using type(None)? In the PEP that form appears a few times. Python might slowly be getting too complicated for my brain
There was a problem hiding this comment.
Experimentally, this form does not seem to work:
In [8]: isinstance(None, None)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
Cell In[8], line 1
----> 1 isinstance(None, None)
TypeError: isinstance() arg 2 must be a type, a tuple of types, or a union
In [9]: isinstance(None, type(None))
Out[9]: True
Have to admit I only got the PEP reference from a google search, and I'm not sure of exact details of whether instances should or should not be allowed as the 2nd isinstance argument, or whether there were post-pep decisions, or if different python versions do different things.
Here type(None) is what just worked so that I did not have to stop looking for alternatives. Apparently, there's from types import NoneType which I just now had to look up where it is defined (and am pretty sure will need to look it up again next time). If type(None) rubs the reader the wrong way, we can switch to that of course.
So yeah, python is getting too complicated for my brain, too :-).
There was a problem hiding this comment.
Ah, here's the thing:
This PEP is a historical document. The up-to-date, canonical documentation can now be found at Union Type.
Looking at the Union Type reference,
int | str == typing.Union[int, str]
and
Optional types can be spelled as a union with None:
str | None == typing.Optional[str]
So int | None and int | type(None) are different. I'm sure there's a convincing argument (~100 comments somewhere) that isinstance receives a Union not Optional. Sigh.
|
I had a quick look at the diff, I think this is going in a direction I like. One thought to which I have no good answer: would it make sense to repurpose Interested what other think about this. |
Add a new device which only supports single precision floats and does not support double precision floats.
The new device mimics torch "mps" device in that it does not have f64 but supports int64---unlike JAX which either has both or none.
dtypes,default_dtypes) device-aware;ones,emptyetc) use the device-specific default dtype when givendtype=None, device=f32_deviceTODO:
fft.{fftfreq, rfftfreq}device=arguments in internal constructionsTBD:
device2or add a newF32_device(if so, bikeshed the name)dtype=float64, device=f32_only_deviceshould raise? torch "mps" tensors raise a TypeError, follow it or mandate a ValueError?Intends to close gh-64,
Gives a way to close gh-38 --- if we have a f32-only device, we probably do not need a global flag
Addresses a large part of gh-70
Cross-ref the spec RFC to allow for missing dtypes , data-apis/array-api#998 --- note that this
array-api-strictPR can only land after the spec is updated;Also cross-ref the test suite tracker data-apis/array-api-tests#431: the test suite is actually fairly far along.